Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build,test: guard eslint with crypto check #26182

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Feb 18, 2019

Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:

$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:
$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/
cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Feb 18, 2019
@danbev
Copy link
Contributor Author

danbev commented Feb 18, 2019

@richardlau
Copy link
Member

Do you know if the test failures have always been like that without intl or is this a recent(ish) regression?

@danbev
Copy link
Contributor Author

danbev commented Feb 18, 2019

Do you know if the test failures have always been like that without intl or is this a recent(ish) regression?

I'm not sure about failures --without-intl but these ones are fairly recent, like in the last week I'd say. Perhaps after the ESLint upgrade (77b39c2).

@cjihrig
Copy link
Contributor

cjihrig commented Feb 18, 2019

It looks like tools/node_modules/eslint/node_modules/file-entry-cache/cache.js indeed just began require()ing crypto (https://github.com/nodejs/node/blame/fd0a861cdb3088f60ee56a8adef05fd50b71f817/tools/node_modules/eslint/node_modules/file-entry-cache/cache.js#L2).

That line seems to have landed upstream two years ago and just made it's way into eslint (https://github.com/royriojas/file-entry-cache/blame/03f700c99b76133dc14648b465a1550ec2930e5c/cache.js#L2).

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 18, 2019
@danbev
Copy link
Contributor Author

danbev commented Feb 21, 2019

Landed in f17f467.

@danbev danbev closed this Feb 21, 2019
@danbev danbev deleted the eslint-crypto-checks branch February 21, 2019 04:42
danbev added a commit that referenced this pull request Feb 21, 2019
Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:
$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/
cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

PR-URL: #26182
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this pull request Feb 21, 2019
Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:
$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/
cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

PR-URL: #26182
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR BridgeAR mentioned this pull request Feb 26, 2019
rvagg pushed a commit that referenced this pull request Feb 28, 2019
Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:
$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/
cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

PR-URL: #26182
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants